feat: Add Mille Solver - #5954
feat: Add Mille Solver #5954goblirsc wants to merge 11 commits into
Conversation
Public API surface diff+22 added, 0 breaking. ➕ Added public APINew types / aliases / enums / variables / concepts (4)
New call signatures (incl. defaulted-arg overloads) (3)
New public data members (15)
|
40c5b6d to
2bea6dd
Compare
|
Two questions to the experts triggered by unit test failures (e.g. lcg_109_alma9_gcc15 (pull_request))
|
9dfa940 to
92d51ec
Compare
0f28665 to
fd24cb2
Compare
|
|
|
||
| namespace ActsPlugins::ActsToMille { | ||
|
|
||
| enum class childProcessStatus { |
There was a problem hiding this comment.
| enum class childProcessStatus { | |
| enum class ChildProcessStatus { |
| ok = 0, | ||
| progNotFound = 1, | ||
| failedRedirectStdout = 2, | ||
| failedWorkDir = 3, | ||
| failedRun = 4, | ||
| unknownError = 5 |
There was a problem hiding this comment.
not 100% sure but I think we usually capitalize these too
| /// @param runDir: If not empty, will run in the specified directory. | ||
| /// Should already exist (caller is responsible). | ||
| /// @return a pair with the call outcome and the return code (if any, else -1). | ||
| childProcessStatus runChildProcess( |
There was a problem hiding this comment.
runChildProcess sounds a bit generic but might be fair inside the ActsPlugins::ActsToMille namespace. the header is placed in detail so I think this function should be too? an alternative name could be runMilleExecutable
| std::string steeringFile = | ||
| "pedeSteerMaster.txt"; /// steering file with run options | ||
| std::filesystem::path workDir = ""; /// directory to run in | ||
| std::vector<std::string> extraOpts = {}; /// extra CLI options | ||
| std::string resFileName = | ||
| ""; /// destination for result file - if empty, keep original | ||
| std::string redirectStdout = ""; /// destination for the cout/cerr printout | ||
| /// from pede - if empty, send to parent | ||
| std::string logFileName = | ||
| ""; /// destination for the log file - if empty, keep original | ||
| std::string histoFileName = | ||
| ""; /// destination for the histogram file - if empty, keep original | ||
| std::string evFileName = | ||
| ""; /// destination for the eigenvector file - if empty, keep original |
There was a problem hiding this comment.
the formatting goes a bit wild here. might be better to lift the comments above the variable like
/// steering file with run options
std::string steeringFile = "pedeSteerMaster.txt";
| }; | ||
|
|
||
| /// @brief package the result of the alignment fit | ||
| struct mpResult { |
There was a problem hiding this comment.
| struct mpResult { | |
| struct MpResult { |
| #if BOOST_VERSION >= 108800 | ||
|
|
||
| #include <boost/process/environment.hpp> | ||
| #include <boost/process/process.hpp> | ||
| #include <boost/process/start_dir.hpp> | ||
| #include <boost/process/stdio.hpp> | ||
|
|
||
| #else | ||
| #include <boost/process.hpp> | ||
| #endif |
There was a problem hiding this comment.
do we support both versions?
| std::filesystem::path m_path{}; | ||
| }; | ||
|
|
||
| #if BOOST_VERSION >= 108800 |
There was a problem hiding this comment.
alternatively we could split this into two headers and put them into the source file, including them depending on the boost version
| // determine where the user wishes to run | ||
| std::filesystem::path workDir = std::filesystem::current_path(); | ||
|
|
||
| if (!runDir.empty()) { | ||
| workDir = runDir; | ||
| } | ||
| if (!std::filesystem::exists(workDir)) { | ||
| std::error_code e; | ||
| std::filesystem::create_directories(workDir, e); | ||
| if (e) { | ||
| return ActsPlugins::ActsToMille::childProcessStatus::failedWorkDir; | ||
| } | ||
| } |
There was a problem hiding this comment.
I think it would be fair to have the workDir as an input and as a precondition that it exists
| ActsPlugins::ActsToMille::runChildProcess( | ||
| const std::string& program, const std::vector<std::string>& args, | ||
| const std::filesystem::path& runDir, | ||
| const std::filesystem::path& output_dest) { |
There was a problem hiding this comment.
| const std::filesystem::path& output_dest) { | |
| const std::filesystem::path& outputDest) { |
| childProcessStatus runChildProcess( | ||
| const std::string& program, const std::vector<std::string>& args, | ||
| const std::filesystem::path& runDir = "", | ||
| const std::filesystem::path& output_dest = ""); |
There was a problem hiding this comment.
| const std::filesystem::path& output_dest = ""); | |
| const std::filesystem::path& outputDest = ""); |



Add a MilleSolver to handle running the MillePede alignment step and collect the results.
Adds a
MilleSolverwhich wraps the call topede. It allows to redirect the output and returns astructwith the fit status and the location of the output files.Currently, it expects the user to have already configured the steering file for the fit - this will be handled by one of the next PRs in the ongoing stack, by a separate tool.
The child process call required for this wrapping is currently done via
Boost::process. For the longer-term future, a refactor ofpedeis foreseen to allow running the alignment fit via an API call to a library, rather than having to invoke an external program. Once that becomes available, this wrapper will be replaced accordingly.The PR also adds python bindings for the solver and its configuration, as well as a unit test.